Pass more information when comparing platforms #3817
Conversation
eregon
force-pushed
the
improve-platform-check
branch
from
51e7307
to
8ea2b52
Compare
| class << self | ||
|
|
||
| extend Gem::Deprecate | ||
| rubygems_deprecate :match, "Gem::Platform.match_spec?" |
There was a problem hiding this comment.
This might be annoying when using Bundler 1, and as long as there is the RubyGems Bundler version switcher (when removed, then Bundler 2+ would always be used and so just having a recent Bundler would avoid the issue).
Any idea how to "soft deprecate" this?
There was a problem hiding this comment.
IMO if you are on a RubyGems new enough to get this deprecation it's ok to tell you to upgrade to Bundler 2? But open to hearing from people that would be a problem for. :)
There was a problem hiding this comment.
One case I can imagine is using e.g. Ruby 2.8/3.0 or update RubyGems (which would include this code),
and then bundle install on a project with Gemfile.lock BUNDLED WITH 1.x.y.
Then Bundler 1 would be used (due to the version switcher) and the warning would be shown.
I think a good way to fix that is to remove the version switcher, so Bundler 2 would be used in that case.
BTW, since it seems Bundler 1 seems no longer maintained (last release of Bundler 1 in 2018), it seems important to use Bundler 2 for all cases if installed, even if the Gemfile.lock still has BUNDLED WITH 1.x.y.
There was a problem hiding this comment.
I filed #3879 about this.
eregon
force-pushed
the
improve-platform-check
branch
from
8ea2b52
to
bad8323
Compare
|
@deivid-rodriguez Could you review this? Do you think we could merge it? |
* Passing the gem specification enables more flexibility when checking whether a gem is compatible with the various aspects of the "current platform" including OS, architecture, RUBY_ENGINE, etc. * Relates to rubygems#2945 * Based on oracle/truffleruby@ae83667
* So it also works with older versions of RubyGems.
eregon
force-pushed
the
improve-platform-check
branch
from
bad8323
to
b9b0758
Compare
There was a problem hiding this comment.
IIRC @deivid-rodriguez is out this week taking a break, and this looks reasonable to me. Let's merge this once it's green, and we can get additional feedback or make additional changes if needed after he gets back.
|
Thanks for merging! |
Pass more information when comparing platforms (cherry picked from commit 8977719)
Pass more information when comparing platforms (cherry picked from commit 8977719)
Pass more information when comparing platforms (cherry picked from commit 8977719)
Although this argument isn't used in the CRuby implementation, other implementations (specifically TruffleRuby) reserve the right to re-implement this method with special cases for specific gems. More context at rubygems/rubygems#3817
when checking whether a gem is compatible
with the various aspects of the "current platform"
including OS, architecture, RUBY_ENGINE, etc.
What was the end-user or developer problem that led to this PR?
As mentioned in #2945 and in other issues (notably about including the libc in the "platform"), the current Gem::Platform captures too little and is sometimes not enough to check if a precompiled extension is actually compatible with the local machine installing the gem.
One example is the
sasscandlibv8gems both have extensions which do not use the Ruby C API and are therefore independent of the Ruby implementation. So such precompiled "extensions" for such gems can be reused on TruffleRuby, JRuby, etc, while typical precompiled C extensions cannot due to having a different ABI.Also consider that while
sassccan be compiled from source, it's "just" rather slow.But for
libv8it's both hard to build (needs dependencies typically not installed by Rubyists) and it takes >15 minutes to compile (oracle/truffleruby#1827 (comment)).What is your fix for the problem, implemented in this PR?
This PR makes a step forwards by giving more information and therefore flexibility when checking whether a Gem's platform is compatible with the local machine.
Tasks:
I will abide by the code of conduct.